Fix: restrict setHeading() to 2D vectors and add friendly error for others #8255
Conversation
perminder-17
left a comment
perminder-17
left a comment
There was a problem hiding this comment.
Really sorry for the delay in the review @reshma045 , I was out of my city.
| const m = this.mag(); | ||
| if (m === 0) { | ||
| this.x = 0; | ||
| this.y = 0; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
I think this block of code is mostly redundant. When m === 0 we had this.x = m * Math.cos(a) which comes out to be this.x = 0 * Math.cos(a) which is this.x = 0, so we can remove this block and revert that change.
| setHeading(a) { | ||
| if (this.isPInst) a = this._toRadians(a); | ||
| let m = this.mag(); | ||
| if (this.z !== 0) { |
There was a problem hiding this comment.
Do you think, this condition is not correct, since the z component of the vector can be zero and it could still be a 3-d vector. Maybe you need to replace this.z !== 0 with this.z !== undefined.
There was a problem hiding this comment.
Also, one more thought. Since setHeading() is logically meaningful only for 2d vectors, so do you think we need to check this.y === undefined as well?
2 participants
Resolves #8215
Changes:
This PR updates the p5.Vector.setHeading() method to support only 2D vectors in alignment with the proposed 2.x design.
setHeading():p5._friendlyError()explaining thatsetHeading()is 2D-only in p5.js 2.x.z === 0), heading behavior remains the same, preserving magnitude and rotating in the xy-plane.setHeading()on true 2D vectors.PR Checklist
npm run lintpasses